Skip to content

Conversation

@jotak
Copy link
Member

@jotak jotak commented Mar 18, 2025

Description

The only visible change here is the "show duplicates" checkbox that is now always displayed (previously it was only visible when deduper.mark was configured in advanced options). Also, it is moved from the Query options dropwdown to the Display options, only in the "Traffic flows" view.

There's also a lot of cleanup here due to agent dedup not being configurable anymore since 1.8 (no more mark/merge, it's always merge)

image

Dependencies

n/a

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

  • Is this PR backed with a JIRA ticket? If so, make sure it is written as a title prefix (in general, PRs affecting the NetObserv/Network Observability product should be backed with a JIRA ticket - especially if they bring user facing changes).
  • Does this PR require product documentation?
    • If so, make sure the JIRA epic is labelled with "documentation" and provides a description relevant for doc writers, such as use cases or scenarios. Any required step to activate or configure the feature should be documented there, such as new CRD knobs.
  • Does this PR require a product release notes entry?
    • If so, fill in "Release Note Text" in the JIRA.
  • Is there anything else the QE team should know before testing? E.g: configuration changes, environment setup, etc.
    • If so, make sure it is described in the JIRA ticket.
  • QE requirements (check 1 from the list):
    • Standard QE validation, with pre-merge tests unless stated otherwise.
    • Regression tests only (e.g. refactoring with no user-facing change).
    • No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Mar 18, 2025

@jotak: This pull request references NETOBSERV-2126 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

In response to this:

Description

Agent dedup isn't a configurable thing anymore since 1.8 (no more mark/merge, it's always merge)
So lots of cleanup here.
The only visible change here should be in query-options-panel.tsx where the "show duplicates" checkbox is visible regardless of deduper.mark

Dependencies

n/a

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

  • Is this PR backed with a JIRA ticket? If so, make sure it is written as a title prefix (in general, PRs affecting the NetObserv/Network Observability product should be backed with a JIRA ticket - especially if they bring user facing changes).
  • Does this PR require product documentation?
  • If so, make sure the JIRA epic is labelled with "documentation" and provides a description relevant for doc writers, such as use cases or scenarios. Any required step to activate or configure the feature should be documented there, such as new CRD knobs.
  • Does this PR require a product release notes entry?
  • If so, fill in "Release Note Text" in the JIRA.
  • Is there anything else the QE team should know before testing? E.g: configuration changes, environment setup, etc.
  • If so, make sure it is described in the JIRA ticket.
  • QE requirements (check 1 from the list):
  • Standard QE validation, with pre-merge tests unless stated otherwise.
  • Regression tests only (e.g. refactoring with no user-facing change).
  • No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci
Copy link

openshift-ci bot commented Mar 18, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jotak for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jotak jotak force-pushed the merge-option branch 2 times, most recently from 1af3115 to 4eb04a0 Compare March 18, 2025 13:45
@jotak jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Mar 18, 2025
@github-actions
Copy link

New image:
quay.io/netobserv/network-observability-console-plugin:a89325d

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=a89325d make set-plugin-image

@jotak jotak requested a review from jpinsonneau March 18, 2025 14:08
@jotak jotak changed the title NETOBSERV-2126: reintroduce merge option, and cleanup old duplicate code NETOBSERV-2126: reintroduce merge option, and cleanup old dedup code Mar 18, 2025
Comment on lines 204 to 228
<div className="pf-v5-c-menu__group">
<Tooltip
content={t(
// eslint-disable-next-line max-len
'A flow might be reported from several interfaces, and from both source and destination nodes, making it appear several times. By default, duplicates are hidden. Showing duplicates is not possible in Overview and Topology tabs to avoid altering metric calculations. Use the Direction filter to switch between ingress, egress and inner-node traffic.'
)}
>
<div className="pf-v5-c-menu__group-title">
<Text component={TextVariants.p}>
{t('Duplicated flows')} <InfoAltIcon />
</Text>
</div>
</Tooltip>
<label className="display-dropdown-padding pf-v5-c-menu__menu-item">
<Checkbox
isChecked={allowShowDuplicates ? showDuplicates : false}
isDisabled={!allowShowDuplicates}
name={'show-duplicates'}
onChange={() => setShowDuplicates(!showDuplicates)}
label={t('Show duplicates')}
data-test={'show-duplicates'}
id={'show-duplicates'}
/>
</label>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we keep that in the query options or move it in the table display options ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done:
image

@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Apr 1, 2025
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Apr 1, 2025

@jotak: This pull request references NETOBSERV-2126 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

In response to this:

Description

The only visible change here is the "show duplicates" checkbox that is now always displayed (previously it was only visible when deduper.mark was configured in advanced options). Also, it is moved from the Query options dropwdown to the Display options, only in the "Traffic flows" view.

There's also a lot of cleanup here due to agent dedup not being configurable anymore since 1.8 (no more mark/merge, it's always merge)

Dependencies

n/a

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

  • Is this PR backed with a JIRA ticket? If so, make sure it is written as a title prefix (in general, PRs affecting the NetObserv/Network Observability product should be backed with a JIRA ticket - especially if they bring user facing changes).
  • Does this PR require product documentation?
  • If so, make sure the JIRA epic is labelled with "documentation" and provides a description relevant for doc writers, such as use cases or scenarios. Any required step to activate or configure the feature should be documented there, such as new CRD knobs.
  • Does this PR require a product release notes entry?
  • If so, fill in "Release Note Text" in the JIRA.
  • Is there anything else the QE team should know before testing? E.g: configuration changes, environment setup, etc.
  • If so, make sure it is described in the JIRA ticket.
  • QE requirements (check 1 from the list):
  • Standard QE validation, with pre-merge tests unless stated otherwise.
  • Regression tests only (e.g. refactoring with no user-facing change).
  • No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@jotak jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Apr 1, 2025
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Apr 1, 2025

@jotak: This pull request references NETOBSERV-2126 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

In response to this:

Description

The only visible change here is the "show duplicates" checkbox that is now always displayed (previously it was only visible when deduper.mark was configured in advanced options). Also, it is moved from the Query options dropwdown to the Display options, only in the "Traffic flows" view.

There's also a lot of cleanup here due to agent dedup not being configurable anymore since 1.8 (no more mark/merge, it's always merge)

image

Dependencies

n/a

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

  • Is this PR backed with a JIRA ticket? If so, make sure it is written as a title prefix (in general, PRs affecting the NetObserv/Network Observability product should be backed with a JIRA ticket - especially if they bring user facing changes).
  • Does this PR require product documentation?
  • If so, make sure the JIRA epic is labelled with "documentation" and provides a description relevant for doc writers, such as use cases or scenarios. Any required step to activate or configure the feature should be documented there, such as new CRD knobs.
  • Does this PR require a product release notes entry?
  • If so, fill in "Release Note Text" in the JIRA.
  • Is there anything else the QE team should know before testing? E.g: configuration changes, environment setup, etc.
  • If so, make sure it is described in the JIRA ticket.
  • QE requirements (check 1 from the list):
  • Standard QE validation, with pre-merge tests unless stated otherwise.
  • Regression tests only (e.g. refactoring with no user-facing change).
  • No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@github-actions
Copy link

github-actions bot commented Apr 1, 2025

New image:
quay.io/netobserv/network-observability-console-plugin:734672d

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=734672d make set-plugin-image

@Amoghrd
Copy link
Member

Amoghrd commented Apr 7, 2025

/retest

@Amoghrd Amoghrd removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Apr 7, 2025
@Amoghrd
Copy link
Member

Amoghrd commented Apr 7, 2025

/ok-to-test

@openshift-ci openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Apr 7, 2025
@github-actions
Copy link

github-actions bot commented Apr 7, 2025

New image:
quay.io/netobserv/network-observability-console-plugin:71317bb

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=71317bb make set-plugin-image

@openshift-ci
Copy link

openshift-ci bot commented Apr 7, 2025

@jotak: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/plugin-cypress a56b31f link true /test plugin-cypress
ci/prow/qe-e2e-console-tests a56b31f link false /test qe-e2e-console-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@Amoghrd
Copy link
Member

Amoghrd commented Apr 7, 2025

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved QE has approved this pull request label Apr 7, 2025
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Apr 7, 2025

@jotak: This pull request references NETOBSERV-2126 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

In response to this:

Description

The only visible change here is the "show duplicates" checkbox that is now always displayed (previously it was only visible when deduper.mark was configured in advanced options). Also, it is moved from the Query options dropwdown to the Display options, only in the "Traffic flows" view.

There's also a lot of cleanup here due to agent dedup not being configurable anymore since 1.8 (no more mark/merge, it's always merge)

image

Dependencies

n/a

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

  • Is this PR backed with a JIRA ticket? If so, make sure it is written as a title prefix (in general, PRs affecting the NetObserv/Network Observability product should be backed with a JIRA ticket - especially if they bring user facing changes).
  • Does this PR require product documentation?
  • If so, make sure the JIRA epic is labelled with "documentation" and provides a description relevant for doc writers, such as use cases or scenarios. Any required step to activate or configure the feature should be documented there, such as new CRD knobs.
  • Does this PR require a product release notes entry?
  • If so, fill in "Release Note Text" in the JIRA.
  • Is there anything else the QE team should know before testing? E.g: configuration changes, environment setup, etc.
  • If so, make sure it is described in the JIRA ticket.
  • QE requirements (check 1 from the list):
  • Standard QE validation, with pre-merge tests unless stated otherwise.
  • Regression tests only (e.g. refactoring with no user-facing change).
  • No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@jotak jotak merged commit 1a8ba1b into netobserv:main Apr 8, 2025
10 of 13 checks passed
jotak added a commit that referenced this pull request May 15, 2025
…775)

* NETOBSERV-2126: reintroduce merge option, and cleanup old duplicate code

* Move 'show duplicates' from query opts to display opts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference lgtm ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. qe-approved QE has approved this pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants